feat(cognitive): improve historian quality and stream LLM metrics - #92
Conversation
Inject merge-time now and profile updated_at so the historian can resolve stale traits against the current input batch. Co-authored-by: Cursor <cursoragent@cursor.com>
Guide lightweight rewrite to prefer verifiable fact sentences with absolute time anchors over speech-act shells. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 29 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change tightens cognitive-memory observation and historian profile rules. It adds provider-aware streaming TTFT/TPS measurement and optional token-usage persistence. Release metadata and documentation now describe version 3.11.1 and these behaviors. ChangesHistorian and streaming metrics
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🔵 Low · up to Profile merging now prioritizes newer facts, but timezone-less timestamps may be mixed with timezone-aware values, making recency selection ambiguous; clarify or normalize this behavior before merging. Sequence Diagram(s)sequenceDiagram
participant Requester
participant StreamingProvider
participant TokenUsageStorage
Requester->>StreamingProvider: Start streaming request
StreamingProvider-->>Requester: Return first meaningful output event
Requester->>Requester: Calculate TTFT and TPS
Requester->>TokenUsageStorage: Persist available metrics
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/message-batching.md`:
- Line 46: Define one consistent observation provenance contract across
docs/message-batching.md:46-46, res/prompts/undefined.xml:240-252 and :961-965,
and res/prompts/undefined_nagaagent.xml:240-252 and :1020-1024. Decide whether
verified assistant-action outcomes are valid observation sources, document that
decision, and update each end.observations and memory-management prompt to
enforce the same rule; preserve the requirement that observations are otherwise
limited to substantive facts from the current input batch and use an empty array
when none exist.
In `@res/prompts/historian_profile_merge.md`:
- Around line 118-135: Update the fenced code block in the positive example to
specify the text language on its opening fence, resolving the MD040 markdownlint
violation while preserving the example content.
In `@res/prompts/historian_rewrite.md`:
- Around line 7-21: Update the examples in the “轻量独立事实” section to use the
required format “QQ号123(昵称张三)” instead of “张三(123)”. Also revise the name-format
rule around line 25 to require cognitive observations to use
“QQ号<user_id>(昵称<name>)”, preserving the explicit numeric identifier.
In `@src/Undefined/ai/llm/streaming.py`:
- Around line 246-266: Update responses_event_marks_ttft so
response.function_call_arguments.delta events return
bool(stringify_stream_delta(event.get("delta"))) instead of unconditionally
marking TTFT. Add a regression test covering an event with an empty string delta
and assert it is not treated as TTFT.
In `@src/Undefined/cognitive/historian/worker.py`:
- Around line 680-705: Update the timestamp setup around now_local_dt,
now_utc_dt, and timezone_label to resolve job["timezone"] with ZoneInfo, using a
safe fallback for invalid or missing values. Derive local and UTC timestamps
from the same current instant, ensure the prompt’s now_local uses the resolved
job timezone, and emit future updated_at values with an explicit offset; apply
the same correction to the corresponding logic near the profile-merge handling
at lines 732-736.
In `@src/Undefined/skills/tools/end/config.json`:
- Line 16: Require user-centered cognitive-memory observations to identify users
as QQ号<sender_id>(昵称<name>), updating the observations description in
src/Undefined/skills/tools/end/config.json:16-16 and the function description at
src/Undefined/skills/tools/end/config.json:5-5. Mirror this stable-ID
requirement in docs/cognitive-memory.md:71-71 and docs/cognitive-memory.md:8-8,
and align src/Undefined/skills/tools/end/README.md:7-7. Extend
tests/test_system_prompt_constraints.py:448-455 and
tests/test_system_prompt_constraints.py:492-498 to assert the rule in both
prompt and end-tool schema contracts.
Apply the same fix in `@docs/cognitive-memory.md` at line 8.
Apply the same fix in `@tests/test_system_prompt_constraints.py` around lines 448
- 455.
Apply the same fix in `@src/Undefined/skills/tools/end/README.md` at line 7.
In `@src/Undefined/token_usage_storage.py`:
- Around line 83-89: Update to_optional_float to return None when the parsed
float is non-finite or negative, while preserving None for null and invalid
inputs and valid non-negative finite values. Add tests covering "NaN",
"Infinity", and negative values.
In `@tests/test_cognitive_historian.py`:
- Around line 375-385: Update the merge template contract test assertions around
the existing now_local check to also require the {now_utc} placeholder and
verify the generated prompt includes a valid UTC timestamp value. Preserve the
existing assertions and add coverage that would fail if the worker stops
supplying or rendering now_utc.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 91f3569e-3c22-43ce-b4f1-30b63193ca66
⛔ Files ignored due to path filters (5)
apps/undefined-chat/package-lock.jsonis excluded by!**/package-lock.jsonapps/undefined-chat/src-tauri/Cargo.lockis excluded by!**/*.lockapps/undefined-console/package-lock.jsonis excluded by!**/package-lock.jsonapps/undefined-console/src-tauri/Cargo.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
AGENTS.mdARCHITECTURE.mdCHANGELOG.mdCLAUDE.mdREADME.mdapps/undefined-chat/package.jsonapps/undefined-chat/src-tauri/Cargo.tomlapps/undefined-chat/src-tauri/tauri.conf.jsonapps/undefined-console/package.jsonapps/undefined-console/src-tauri/Cargo.tomlapps/undefined-console/src-tauri/tauri.conf.jsondocs/cognitive-memory.mddocs/configuration.mddocs/message-batching.mddocs/model-compatibility.mddocs/usage.mdpyproject.tomlres/prompts/historian_profile_merge.mdres/prompts/historian_rewrite.mdres/prompts/undefined.xmlres/prompts/undefined_nagaagent.xmlsrc/Undefined/__init__.pysrc/Undefined/ai/llm/requester.pysrc/Undefined/ai/llm/streaming.pysrc/Undefined/cognitive/historian/helpers.pysrc/Undefined/cognitive/historian/worker.pysrc/Undefined/skills/tools/end/README.mdsrc/Undefined/skills/tools/end/config.jsonsrc/Undefined/token_usage_storage.pytests/test_cognitive_historian.pytests/test_llm_streaming.pytests/test_system_prompt_constraints.pytests/test_token_usage_unit.py
Keep observation provenance, QQ-ID naming, job timezone offsets, and TTFT/TPS parsing consistent so merge prompts and usage logs stay trustworthy. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_cognitive_historian.py (1)
403-411: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCover malformed and non-dictionary frontmatter.
The helper now promises empty results for malformed YAML and non-dictionary YAML. These tests cover missing frontmatter and a missing
updated_atfield, but not those two fallback paths. Add both cases.Based on learnings, add or update test coverage for behavior changes in cognitive memory.
Suggested cases
+ assert _extract_frontmatter_updated_at("---\n- item\n---") == "" + assert _extract_frontmatter_updated_at("---\nupdated_at: [\n---") == ""🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cognitive_historian.py` around lines 403 - 411, Add cases to test_extract_frontmatter_updated_at for malformed YAML and valid YAML whose top-level value is not a dictionary, asserting both return an empty string. Keep the existing timestamp, missing-field, and absent-frontmatter assertions unchanged.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_cognitive_historian.py`:
- Around line 423-428: Update test_now_in_job_timezone_invalid_falls_back to
compare now_local’s UTC offset and timezone name against
datetime.now().astimezone(), asserting the invalid timezone falls back to the
system-local timezone rather than merely checking the label differs. Retain the
existing UTC and consistency assertions.
---
Outside diff comments:
In `@tests/test_cognitive_historian.py`:
- Around line 403-411: Add cases to test_extract_frontmatter_updated_at for
malformed YAML and valid YAML whose top-level value is not a dictionary,
asserting both return an empty string. Keep the existing timestamp,
missing-field, and absent-frontmatter assertions unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e4c180b-fd8b-4160-9ca1-15ebc4aa9470
📒 Files selected for processing (16)
docs/cognitive-memory.mddocs/message-batching.mdres/prompts/historian_profile_merge.mdres/prompts/historian_rewrite.mdres/prompts/undefined.xmlres/prompts/undefined_nagaagent.xmlsrc/Undefined/ai/llm/streaming.pysrc/Undefined/cognitive/historian/helpers.pysrc/Undefined/cognitive/historian/worker.pysrc/Undefined/skills/tools/end/README.mdsrc/Undefined/skills/tools/end/config.jsonsrc/Undefined/token_usage_storage.pytests/test_cognitive_historian.pytests/test_llm_streaming.pytests/test_system_prompt_constraints.pytests/test_token_usage_unit.py
💤 Files with no reviewable changes (1)
- src/Undefined/ai/llm/streaming.py
🚧 Files skipped from review as they are similar to previous changes (12)
- res/prompts/undefined.xml
- src/Undefined/skills/tools/end/config.json
- tests/test_token_usage_unit.py
- src/Undefined/skills/tools/end/README.md
- tests/test_system_prompt_constraints.py
- docs/cognitive-memory.md
- src/Undefined/cognitive/historian/worker.py
- tests/test_llm_streaming.py
- res/prompts/historian_profile_merge.md
- res/prompts/undefined_nagaagent.xml
- src/Undefined/token_usage_storage.py
- res/prompts/historian_rewrite.md
Assert invalid job timezones fall back to the system-local zone, and cover malformed or non-mapping YAML when reading profile updated_at. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
end.observations:只写值得日后检索的写实内容,宁缺毋滥,禁止流程决策、否定清单和碎碎念。Test plan
end.observations为空,不会硬凑静默决策/否定清单data/token_usage.jsonl含ttft_seconds/tokens_per_second;非流式记录不含这两项uv run pytest tests/test_cognitive_historian.py tests/test_llm_streaming.py tests/test_token_usage_unit.py tests/test_system_prompt_constraints.pyMade with Cursor
Summary by CodeRabbit
New Features
Improvements
Documentation
Release